Skip to content

Conversation

@ameliahsu
Copy link
Member

fixes a bug where X-Hits was turning the total sum of alerts (right column), rather than the total number of groups (rows in the table)

image (2)

@ameliahsu ameliahsu requested a review from a team as a code owner November 14, 2025 20:21
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Nov 14, 2025
if known_hits is not None:
hits = known_hits
elif count_hits:
hits = self.count_hits(max_hits=MAX_HITS_LIMIT)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it doesn't seem like we're using the max_hits variable from the method signature, should that be used here if set?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good callout, updated in 3da9532

)

# Count distinct groups for pagination
group_count = qs.count()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a quick note that this will add another query and may impact performance; i'm guessing we need this to get the total number of results compared to the paginated list though.

elif count_hits:
if max_hits is None:
max_hits = MAX_HITS_LIMIT
hits = self.count_hits(max_hits)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Inconsistent Paginator Hit Counts

When known_hits is provided, it should be capped by max_hits for consistency with SequencePaginator. Currently, OffsetPaginator uses known_hits directly without applying the max_hits bound. If both parameters are provided together, hits could exceed max_hits, creating inconsistent behavior across paginators.

Fix in Cursor Fix in Web

@ameliahsu ameliahsu merged commit ea5fdbd into master Nov 18, 2025
66 checks passed
@ameliahsu ameliahsu deleted the mia/aci/fix-workflow-history-hits branch November 18, 2025 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants